Update generated formulas to follow the new naming scheme by sahas-subramanian-frequenz · Pull Request #350 · frequenz-floss/frequenz-sdk-python

sahas-subramanian-frequenz · 2023-04-20T10:53:18Z

New power streaming methods - public api

BatteryPool.{production_power, consumption_power}
EVChargerPool.{production_power, consumption_power}
LogicalMeter:
- consumer_power
- grid_production_power
- grid_consumption_power
- chp_power
- chp_production_power
- chp_consumption_power (probably not necessary, but having just a production method was a bit weird)

New FormulaEngine steps - internal

ConstantValue - for multiplying formula outputs with -1 to get production power.
Clipper - for clipping formula outputs to 0.0 in production/consumption power formulas.

New component graph methods - internal

is_pv_inverter, is_pv_meter, is_pv_chain
is_battery_inverter, is_battery_meter, is_battery_chain
is_ev_charger, is_ev_charger_meter, is_ev_charger_chain,
is_chp, is_chp_meter, is_chp_chain

These methods were particularly useful in the creation of the consumer_power formula.

The *chain methods return true if either of the other corresponding methods return true. Maybe there's a better name, but it is for internal use.

sahas-subramanian-frequenz · 2023-04-24T16:05:20Z

Need to add tests for the consumer_power formula, will be ready tomorrow.

cwasicki

Some high-level comments. Most importantly I think the methods for solar are missing.

sahas-subramanian-frequenz · 2023-05-05T10:21:07Z

Rebased on latest.

matthias-wende-frequenz

Almost finished. ~~I've a couple of open question w.r.t. the consumer power.~~ No questions. I missed the continue in the loop over the successors :).

matthias-wende-frequenz · 2023-05-05T11:58:54Z

+                    f"{meter.category}. Expected ComponentCategory.METER."
+                )
+            meter_successors = component_graph.successors(meter.component_id)
+            if len(meter_successors) != 1:


Is that check necessary? Seems to me that we would disallow a valid grid topology, where one meter is measuring more then 1 CHPs?

true, updated.

matthias-wende-frequenz · 2023-05-05T13:40:32Z

+
+    @abstractmethod
+    def is_battery_chain(self, component: Component) -> bool:
+        """Check if the specified component is part of a battery chain.


I suggest to use the docs to define what a battery chain is.

matthias-wende-frequenz · 2023-05-05T13:41:10Z

+
+    @abstractmethod
+    def is_ev_charger_chain(self, component: Component) -> bool:
+        """Check if the specified component is part of an EV charger chain.


matthias-wende-frequenz · 2023-05-05T13:41:24Z

+
+    @abstractmethod
+    def is_chp_chain(self, component: Component) -> bool:
+        """Check if the specified component is part of a CHP chain.


matthias-wende-frequenz · 2023-05-05T15:52:01Z

+            return self._gen_with_grid_meter(builder, grid_meter)
+        return self._gen_without_grid_meter(builder, grid_successors)
+
+    def _gen_with_grid_meter(


We need to be careful here since this stops working once we add sub panels (Unterverteilungen) into the microgrid api. Maybe it's worth adding a comment here.

Are we going to introduce sub panels into the component graph? I haven't heard of any such plans. Those are passive components right? i.e we can't read from them or control them.

As far as I remember that was the plan. @tiyash-basu-frequenz can you confirm?

I suggest to add a short warning comment to the documentation such that we are aware in the future.

I have added a comment:

If the component graph supports additional types of grid successors in the future, additional checks need to be added here.

I think the term "sub panels" is misleading. If I have understood it correctly, then it is a section of the microgrid behind a dedicated fuse. I'd call it sub-section.

But I'd suggest ignoring it for now. It is not scheduled to be added to the microgrid API soon. To me even a comment seems unnecessary: because it does not improve the readability and just hints at a future functionality without knowing when we will add it. But up to you.

If you really want to track this feature, then creating an issue is the proper way to do it.

You are right w.r.t. the issue. We should create one but I'd also leave a comment just to have the behavior documented because it tells that the algorithm isn't going into the depth.

Then I'd suggest to rather create the issue and add a link to the issue in the comment.

I'd suggest doing it in moderation, if never at all, because you do not want to end up adding every proposed feature in the comments.

Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

This is useful for switching the sign of values, by multiplying by -1, for example. Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

matthias-wende-frequenz

Approved apart from my last comment.

Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

The battery power formula has been moved to the battery pool, and is being tested there. And a new test for the new PV Power methods was added in the previous commit. Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

sahas-subramanian-frequenz requested a review from a team as a code owner April 20, 2023 10:53

sahas-subramanian-frequenz requested a review from ela-kotulska-frequenz April 20, 2023 10:53

sahas-subramanian-frequenz marked this pull request as draft April 20, 2023 10:53

sahas-subramanian-frequenz force-pushed the new-power-formulas branch 2 times, most recently from fd75584 to 068f61b Compare April 24, 2023 16:03

github-actions Bot removed part:actor Affects an actor ot the actors utilities (decorator, etc.) part:docs Affects the documentation part:config Affects the configuration management labels Apr 24, 2023

sahas-subramanian-frequenz force-pushed the new-power-formulas branch 4 times, most recently from fa71205 to f6fb8cb Compare April 25, 2023 15:07

sahas-subramanian-frequenz self-assigned this Apr 25, 2023

sahas-subramanian-frequenz added this to the v0.21.0 milestone Apr 25, 2023

sahas-subramanian-frequenz marked this pull request as ready for review April 25, 2023 15:56

sahas-subramanian-frequenz requested a review from cwasicki April 25, 2023 15:56

sahas-subramanian-frequenz linked an issue Apr 25, 2023 that may be closed by this pull request

Update generated formulas to use new naming scheme #340

Closed

sahas-subramanian-frequenz force-pushed the new-power-formulas branch from f6fb8cb to 11363e5 Compare April 26, 2023 12:07

cwasicki reviewed Apr 27, 2023

View reviewed changes

sahas-subramanian-frequenz force-pushed the new-power-formulas branch 3 times, most recently from 3c8d3c1 to 21e6e6a Compare May 2, 2023 10:22

sahas-subramanian-frequenz requested a review from cwasicki May 2, 2023 12:29

sahas-subramanian-frequenz force-pushed the new-power-formulas branch from f9f6884 to 72ee63c Compare May 5, 2023 10:17

matthias-wende-frequenz reviewed May 6, 2023

View reviewed changes

sahas-subramanian-frequenz force-pushed the new-power-formulas branch 2 times, most recently from 13e7fe1 to 7c30e4c Compare May 9, 2023 15:33

shsms added 10 commits May 15, 2023 12:03

Replace type:ignores with type asserts in formula generation methods

ab1de11

Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

Remove unused logger variable from _logical_meter.py

4a5e11e

Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

Add formula step for applying constant values

9970163

This is useful for switching the sign of values, by multiplying by -1, for example. Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

Add a Clipper formula step for clipping formula output to a range

afbffed

Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

Add a FormulaType parameter for FormulaGeneratorConfig

c8fb3d7

Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

Add grid_{production,consumption}_power formulas to LogicalMeter

53efded

Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

Add LogicalMeter.chp_{,consumption,production}power methods

eb2bfb6

Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

Add BatteryPool.{production,consumption}_power methods

db77b3c

Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

Add EVChargerPool.{production,consumption}_power methods

ae084f9

Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

Add component graph methods to check component types

c198ad7

Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

sahas-subramanian-frequenz force-pushed the new-power-formulas branch from 7c30e4c to bb1ece8 Compare May 15, 2023 10:03

matthias-wende-frequenz previously approved these changes May 15, 2023

View reviewed changes

shsms added 4 commits May 15, 2023 17:15

Add formula generator for consumer power

26eea48

Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

Add LogicalMeter.pv_{production,consumption}_power methods

b70d414

Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

Remove redundant test from logical meter

ec3b974

The battery power formula has been moved to the battery pool, and is being tested there. And a new test for the new PV Power methods was added in the previous commit. Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

Add formula power/current sign convention details to docstrings

d3d6b31

Signed-off-by: Sahas Subramanian <sahas.subramanian@frequenz.com>

shsms dismissed matthias-wende-frequenz’s stale review via d3d6b31 May 15, 2023 15:15

shsms force-pushed the new-power-formulas branch from bb1ece8 to d3d6b31 Compare May 15, 2023 15:15

shsms assigned shsms and unassigned sahas-subramanian-frequenz May 15, 2023

matthias-wende-frequenz approved these changes May 15, 2023

View reviewed changes

shsms added this pull request to the merge queue May 15, 2023

Merged via the queue into frequenz-floss:v0.x.x with commit 3edf6a5 May 15, 2023

sahas-subramanian-frequenz deleted the new-power-formulas branch May 16, 2023 14:11

Uh oh!

Conversation

sahas-subramanian-frequenz commented Apr 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sahas-subramanian-frequenz commented Apr 24, 2023

Uh oh!

cwasicki left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sahas-subramanian-frequenz commented May 5, 2023

Uh oh!

matthias-wende-frequenz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matthias-wende-frequenz left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sahas-subramanian-frequenz commented Apr 20, 2023 •

edited

Loading

matthias-wende-frequenz left a comment •

edited

Loading